Restructure Unit/Property Model Imports; Eliminate Duplicate Prop Model Names #1693

kurbansitterley · 2025-11-21T03:27:25Z

Fixes/Resolves:

NA

Summary/Motivation:

Currently importing a property model requires also knowing the name of the actual file the parameter block lives in, which is often non-intuitive and cumbersome. Additionally, we have several property models that have the same named parameter block, which is also non-intuitive and cumbersome (and confusing). The same is true for unit models.

This PR modifies the init so you can:

from watertap.property_models import __________

Or for unit models:

from watertap.unit_models import ________

We also have a few property models that share a name:

NaClParameterBlock is shared by three property models
WaterParameterBlock is shared by two property models

This PR also renames them to the following:

NaClParameterBlock for crystallizer is CrystallizerParameterBlock
NaClParameterBlock for the temperature dependent property model is NaClTDepParameterBlock
WaterParameterBlock for the ZO property model is now ZOParameterBlock

This PR also cleaned up and improves the import structure in many unit files to follow

built-in imports

pyomo imports

idaes imports

watertap imports

Imports are updated where necessary. Docs are updated where necessary.

Changes proposed in this PR:

restructure property package imports
add all unit models to unit_models init
eliminate duplicate names of property models to be more intuitive and rename tests to reflect new names
move ZO property package to property_models and rename to zero_order_prop_pack
move, improve, and correct ZO property model docs to property model section of docs
add import headers to all prop model doc files
update/improve/clean up imports where necessary in entire package
update docs to reflect all changes

TO DO:

continue updating imports where necessary
update docs to reflect changes where necessary

Legal Acknowledgement

By contributing to this software project, I agree to the following terms and conditions for my contribution:

I agree my contributions are submitted under the license terms described in the LICENSE.txt file at the top level of this directory.
I represent I am authorized to make the contributions and grant the license. If my employer has rights to intellectual property that includes these contributions, I represent that I have received permission to make contributions and grant the required license on behalf of that employer.

adam-a-a · 2025-11-21T16:51:21Z

docs/technical_reference/core/water_props.rst

 ======================

 .. index::
   pair: watertap.core.zero_order_properties;WaterParameterBlock


Suggested change

pair: watertap.core.zero_order_properties;ZOParameterBlock

SHould this pair be updated too?

yeah docs are still a TO DO

adam-a-a · 2025-11-21T16:52:46Z

docs/technical_reference/core/water_props.rst


 .. currentmodule:: watertap.core.zero_order_properties

 The ideal water properties module contains a simple property package for saline waters which is intended for use with the WaterTap zero-order unit model library. The ideal water property package can be used in a flowsheet as shown below:


Suggested change

The ideal water properties module (zero-order property model) contains a simple property package for saline waters which is intended for use with the WaterTap zero-order unit model library. The ideal water property package can be used in a flowsheet as shown below:

Shouldn't we update the description a little bit?

Yes definitely. I would argue we should drop the "ideal properties" language entirely

This doc was re-written and moved.

adam-a-a · 2025-11-21T17:02:48Z

watertap/flowsheets/generic_desalination_train/generic_train.py

+    MCASParameterBlock,
+    ActivityCoefficientModel,
+    DensityCalculation,
+    MaterialFlowBasis,


No big deal, but I noticed that in another file change, you switched to importing MaterialFlowBasis from idaes.core. That said, I prefer having MaterialFlowBasis importable from watertap.property_models too for convenience.

I agree and I was going to do this but thought I would get pushback because it would require doing from idaes.core import MaterialFlowBasis in the init and I am not sure if that is a packaging no-no. But I can add it

adam-a-a · 2025-11-21T17:05:24Z

watertap/flowsheets/seawater_RO_desalination/seawater_RO_desalination.py


 from watertap.core.wt_database import Database
-import watertap.core.zero_order_properties as prop_ZO
+import watertap.property_models.zero_order_properties as prop_ZO


just a note on consistency---in other files, you do import ZOParameterBlock--but not here.

Yeah haven't finished cleaning things up yet. I intend to be consistent across all files.

adam-a-a · 2025-11-23T13:55:19Z

One issue I thought of with bringing this PR in during the Academy--if we'd like them to bring in any latest changes from main before the course concludes, they'd get whatever those targeted changes are, plus all of this, and I wonder if that'd throw them off (say, if imports start breaking.

kurbansitterley · 2025-11-23T22:09:52Z

One issue I thought of with bringing this PR in during the Academy--if we'd like them to bring in any latest changes from main before the course concludes, they'd get whatever those targeted changes are, plus all of this, and I wonder if that'd throw them off (say, if imports start breaking.

I am not sure what you mean here -- in theory if this was merged, imports wouldn't be breaking... Also the previous import structure still works fine.

But either way yeah this PR really got away from me, if we want to hold off on it that is ok with me. I would be interested in what real developers think of a PR that modifies >300 files @sufikaur @ksbeattie @dangunter. I kept thinking "this could create a ton of merge conflicts for a ton of people"

adam-a-a · 2025-11-23T22:42:25Z

One issue I thought of with bringing this PR in during the Academy--if we'd like them to bring in any latest changes from main before the course concludes, they'd get whatever those targeted changes are, plus all of this, and I wonder if that'd throw them off (say, if imports start breaking.

I am not sure what you mean here -- in theory if this was merged, imports wouldn't be breaking... Also the previous import structure still works fine.

But either way yeah this PR really got away from me, if we want to hold off on it that is ok with me. I would be interested in what real developers think of a PR that modifies >300 files @sufikaur @ksbeattie @dangunter. I kept thinking "this could create a ton of merge conflicts for a ton of people"

This can lead to merge conflicts, for one. Two, if you want to be absolutely sure nothing will break, then you should put up a PR on the academy repo that checks that no notebooks will fail when pointing to this PR. Maybe nothing breaks on the academy repo —but you cannot assure that simply because you have tests passing here.

adam-a-a · 2025-11-23T23:26:11Z

One issue I thought of with bringing this PR in during the Academy--if we'd like them to bring in any latest changes from main before the course concludes, they'd get whatever those targeted changes are, plus all of this, and I wonder if that'd throw them off (say, if imports start breaking.

I am not sure what you mean here -- in theory if this was merged, imports wouldn't be breaking... Also the previous import structure still works fine.
But either way yeah this PR really got away from me, if we want to hold off on it that is ok with me. I would be interested in what real developers think of a PR that modifies >300 files @sufikaur @ksbeattie @dangunter. I kept thinking "this could create a ton of merge conflicts for a ton of people"

This can lead to merge conflicts, for one. Two, if you want to be absolutely sure nothing will break, then you should put up a PR on the academy repo that checks that no notebooks will fail when pointing to this PR. Maybe nothing breaks on the academy repo —but you cannot assure that simply because you have tests passing here.

Here's an expected one to anticipate:

---------------------------------------------------------------------------
ImportError                               Traceback (most recent call last)
Cell In[1], line 12
     10 from watertap.property_models.seawater_prop_pack import SeawaterParameterBlock
     11 from watertap.property_models.NaCl_prop_pack import NaClParameterBlock
---> 12 from watertap.property_models.NaCl_T_dep_prop_pack import (
     13     NaClParameterBlock as NaClParameterBlock_Tdep,
     14 )
     15 from watertap.core.solvers import get_solver

ImportError: cannot import name 'NaClParameterBlock' from 'watertap.property_models.NaCl_T_dep_prop_pack' (my_root\watertap\watertap\property_models\NaCl_T_dep_prop_pack.py)

…tion

…packs

kurbansitterley added 25 commits November 20, 2025 19:13

first init mod

c3ac42c

add __all__

e9f56df

rename cryst prop pack

315ead4

rename NaCl T dep

aadb413

fix imports coag prop pack

4fc5574

fix import and rename cryst tests

daacba6

fix MCAS import

5fd847a

fix NaCl import

7183065

fix NaCl tdep import; rename tests

71ae1ae

fix NDMA import

b137f16

fix sw import

46d12a4

fix water import

858efa3

rename and move ZO prop pack

fe7aa8c

rename cryst prop pack

978e2f9

result of moving and renaming ZO prop pack

ade8656

rename zo prop pack

961c5a9

prop pack init

c985687

fix sim cryst prop pack import

ec9bbba

fix dye desal fs imports

188d3bf

fix ed fs imports

c3ed4a0

fix gac and generic train imports

9538cd3

fix ix fs imports

6922e91

change lsrro imports

cc667f9

black

a1416ea

more black

7e7564f

kurbansitterley self-assigned this Nov 21, 2025

adam-a-a reviewed Nov 21, 2025

View reviewed changes

kurbansitterley added 18 commits November 22, 2025 12:34

finish test imports

99d53ba

revert lsrro

1e7006e

remove unit model config enums

85f72be

update how to run ZO model

08c79d8

update how to use prop model

f19dff1

update how to setup simple RO

173a3b0

update how to setup RO config

7596984

update how to use MCAS

3ddac00

remove duplicate pyunits import

4f8c583

update how to use property test harness

46ac1b1

update how to use test harness

2759466

update unit model doc headers

b0261bd

add import headers to prop model docs

38f4c11

rename ZO properties to ZO prop pack

b47e81e

update init for zo prop pack

73bfa84

add zo prop pack docs and update/correct

fd719a9

update imports in docs

354c538

random import updates

a43e6d5

kurbansitterley changed the title ~~Restructure Property Model Imports and Eliminate Duplicate Prop Model Names~~ Restructure Unit/Property Model Imports; Eliminate Duplicate Prop Model Names Nov 22, 2025

kurbansitterley added 2 commits November 22, 2025 13:40

correct separator import

5dc0f3d

correct ED 1stack fs config import

415f59b

kurbansitterley added 2 commits November 23, 2025 15:12

drop the __all__ for linting

0d63807

new line after end of section

00e2f36

ksbeattie added the Priority:High High Priority Issue or PR label Dec 4, 2025

Sufi Kaur added 2 commits December 18, 2025 15:55

add academy testing and comment out rest of tests for speedy confirma…

c5fce52

…tion

Merge remote-tracking branch 'watertap-origin/main' into import-prop-…

705df08

…packs


		.. currentmodule:: watertap.core.zero_order_properties

		The ideal water properties module contains a simple property package for saline waters which is intended for use with the WaterTap zero-order unit model library. The ideal water property package can be used in a flowsheet as shown below:


	The ideal water properties module (zero-order property model) contains a simple property package for saline waters which is intended for use with the WaterTap zero-order unit model library. The ideal water property package can be used in a flowsheet as shown below:

Restructure Unit/Property Model Imports; Eliminate Duplicate Prop Model Names #1693

Are you sure you want to change the base?

Restructure Unit/Property Model Imports; Eliminate Duplicate Prop Model Names #1693

Uh oh!

Conversation

kurbansitterley commented Nov 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Fixes/Resolves:

Summary/Motivation:

Changes proposed in this PR:

TO DO:

Legal Acknowledgement

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adam-a-a commented Nov 23, 2025

Uh oh!

kurbansitterley commented Nov 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

adam-a-a commented Nov 23, 2025

Uh oh!

adam-a-a commented Nov 23, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

kurbansitterley commented Nov 21, 2025 •

edited

Loading

kurbansitterley commented Nov 23, 2025 •

edited

Loading